Fixed what-if functionality with -QueryString parameter by Xynoclafe · Pull Request #14328 · Azure/azure-powershell

Xynoclafe · 2021-02-24T20:45:24Z

Description

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate ChangeLog.md file(s) has been updated:
- For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
- A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
  - {Please put the link here}
- the markdown help files have been regenerated using the commands listed here

shenglol · 2021-02-25T01:19:36Z

src/Resources/ResourceManager/SdkModels/Deployments/PSDeploymentWhatIfCmdletParameters.cs

+
+                if (!string.IsNullOrEmpty(this.QueryString))
+                {
+                    properties.TemplateLink.QueryString = this.QueryString;


Can you add a test for this?

Pushed a test case

shenglol

shenglol

Looks good to me but still need reviews from the PS team.

Xynoclafe · 2021-02-26T19:09:21Z

Looks good to me but still need reviews from the PS team.

There are some credscan failures that I am trying to fix

Xynoclafe · 2021-02-26T22:38:41Z

Looks good to me but still need reviews from the PS team.

There are some credscan failures that I am trying to fix

The credscan issues are false positives as the credentials are invalid and are for non existent resources.

Xynoclafe · 2021-03-02T22:21:58Z

The credscan issues have been suppressed.
Unsure of the reason for test failures. The test fails claiming "Remove-AzStorageAccount" is not recognized as a cmdlet, which is weird. It is a valid cmdlet (https://docs.microsoft.com/en-us/powershell/module/az.storage/Remove-azStorageAccount?view=azps-5.5.0) and I was able to record the test successfully as well. So I don't know why a successful test record is failing during the checks. I would appreciate it if the reviewers could help if they've seen behavior like this before.

wyunchi-ms · 2021-03-04T14:50:12Z

src/Resources/Resources.Test/ScenarioTests/ResourceTestRunner.cs

                {
                    helper.RMResourceModule,
-                    helper.GetRMModulePath("AzureRM.Monitor.psd1")
+                    helper.GetRMModulePath("AzureRM.Monitor.psd1"),


Add helper.GetRMModulePath("AzureRM.Storage.psd1"), here to use cmdlets in Storage when running test cases.

@wyunchi-ms I made the suggested changes, but now it fails with "Device not configured"

wyunchi-ms · 2021-03-05T02:29:11Z

src/Resources/Resources.Test/ScenarioTests/DeploymentTests.ps1

+		$token = New-AzStorageAccountSASToken -Service File -ResourceType Service,Container,Object -Permission "r" -Context $context -ExpiryTime (Get-Date).AddMinutes(3)
+
+		#Create deployment
+		$deployment =New-AzResourceGroupDeployment -Name $rname -ResourceGroupName $rgname -TemplateUri "https://querystringpstests.file.core.windows.net/querystringshare/sampleLinkedTemplateParent.json" -QueryString $token.Substring(1)


Is this URL valid? I cannot access it in my environment.

Same reasoning as the comment below

wyunchi-ms · 2021-03-05T02:29:16Z

src/Resources/Resources.Test/ScenarioTests/DeploymentTests.ps1

+		Assert-AreEqual Succeeded $deployment.ProvisioningState
+
+		#Run What-if
+		$result =  New-AzResourceGroupDeployment -Name $rname -ResourceGroupName $rgname -TemplateUri "https://querystringpstests.file.core.windows.net/querystringshare/sampleLinkedTemplateParent.json" -QueryString $token.Substring(1) -WhatIf


Is this URL valid? I cannot access it in my environment.

Same reasoning as the comment below

wyunchi-ms · 2021-03-05T02:29:21Z

src/Resources/Resources.Test/ScenarioTests/DeploymentTests.ps1

+
 		#Create deployment
-		$deployment =New-AzResourceGroupDeployment -Name $rname -ResourceGroupName $rgname -TemplateUri "https://querystringtesting.blob.core.windows.net/testqsblob/linkedTemplateParent.json" -QueryString "foo"
+		$deployment =New-AzResourceGroupDeployment -Name $rname -ResourceGroupName $rgname -TemplateUri "https://querystringpstests.file.core.windows.net/querystringshare/sampleLinkedTemplateParent.json" -QueryString $token.Substring(1)


Is this URL valid? I cannot access it in my environment.

They are urls for a storage account created during the test and deleted after it is completed. So you will not be able to access it outside of the test. But at the time of the test execution (recording), the urls are valid.

@wyunchi-ms any idea on what is happening with this error?

…to xynoclafe/whatIfQueryString

Xynoclafe · 2021-03-12T01:42:50Z

The checks should be passing now. @wyunchi-ms

…to xynoclafe/whatIfQueryString

Xynoclafe · 2021-03-12T16:00:15Z

The checks are either not starting or just getting stuck in running (even though the actual pipelines for them seem to have run successfully). Is there something wrong with the checks/pipeline?
Also, I see that credscan is failing. It seems like it is coming from this file - https://github.com/Xynoclafe/azure-powershell/blob/xynoclafe/whatIfQueryString/src/CloudService/test/New-AzCloudService.Recording.json, based on https://dev.azure.com/azure-sdk/internal/_build/results?buildId=780021&view=logs&j=275f1d19-1bd8-5591-b06b-07d489ea915a&t=a4822bc1-f3a9-5984-c941-6c1b7aa2ab81&l=18.
This was last modified in this PR - #14360 and was not modified in the current PR.
cc- @wyunchi-ms

* update readme * Update README.md

Xynoclafe · 2021-03-16T16:29:44Z

The earlier check failures were due to some merge issues with the credScanSuppression file. The checks are passing now, we should be able to merge this.

…ic parameters.

wyunchi-ms · 2021-03-17T07:21:21Z

/azp run

azure-pipelines · 2021-03-17T07:21:58Z

Azure Pipelines successfully started running 3 pipeline(s).

Fixed what-if funcationality with -QueryString parameter

dcff842

Xynoclafe requested a review from shenglol February 24, 2021 21:16

shenglol reviewed Feb 25, 2021

View reviewed changes

Added/Modified test cases for -QueryString

c608cd3

Xynoclafe marked this pull request as draft February 26, 2021 17:56

shenglol approved these changes Feb 26, 2021

View reviewed changes

shenglol previously approved these changes Feb 26, 2021

View reviewed changes

Suppress credscan (credentials are invalid/for deleted resources)

39c53f0

Xynoclafe dismissed shenglol’s stale review via 39c53f0 March 2, 2021 00:51

Xynoclafe marked this pull request as ready for review March 2, 2021 22:14

wyunchi-ms reviewed Mar 4, 2021

View reviewed changes

include storage module in tests

7731a98

wyunchi-ms reviewed Mar 5, 2021

View reviewed changes

wyunchi-ms assigned Xynoclafe, shenglol and wyunchi-ms Mar 10, 2021

wyunchi-ms added the needs-revision label Mar 10, 2021

Xynoclafe added 3 commits March 11, 2021 17:39

Remove scenario test and add mock test for -QueryString scenarios

313600d

Merge branch 'master' of https://github.com/Azure/azure-powershell in…

3fe7d81

…to xynoclafe/whatIfQueryString

Update changelog

971ec2c

Xynoclafe marked this pull request as draft March 11, 2021 23:12

Fix build issues; Switch a mock test for a scenario test

7df85e4

Xynoclafe marked this pull request as ready for review March 12, 2021 01:42

Merge branch 'master' of https://github.com/Azure/azure-powershell in…

1584bea

…to xynoclafe/whatIfQueryString

Merge branch 'master' into xynoclafe/whatIfQueryString

0f681c6

dingmeng-xue and others added 4 commits March 16, 2021 20:22

Replace myget with Azure artifacts (Azure#14543)

fa42dd3

Fix credscan file

18614a8

Update Cloud Shell image in README.md (Azure#14531)

36d8201

* update readme * Update README.md

Fix merge

9639ca0

Xynoclafe changed the title ~~Fixed what-if funcationality with -QueryString parameter~~ Fixed what-if functionality with -QueryString parameter Mar 16, 2021

Normalized -QueryString starting with ? for scenarios involving dynam…

acaf849

…ic parameters.

wyunchi-ms changed the base branch from master to release-2021-03-23 March 17, 2021 05:05

wyunchi-ms added 2 commits March 17, 2021 13:05

Merge branch 'release-2021-03-23' into xynoclafe/whatIfQueryString

45c4600

Merge branch 'release-2021-03-23' into xynoclafe/whatIfQueryString

8853e83

Merge branch 'release-2021-03-23' into xynoclafe/whatIfQueryString

dc1a2f5

wyunchi-ms approved these changes Mar 17, 2021

View reviewed changes

isra-fel merged commit 3907ba0 into Azure:release-2021-03-23 Mar 17, 2021

Conversation

Xynoclafe commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shenglol left a comment

Choose a reason for hiding this comment

Uh oh!

shenglol left a comment

Choose a reason for hiding this comment

Uh oh!

Xynoclafe commented Feb 26, 2021

Uh oh!

Xynoclafe commented Feb 26, 2021

Uh oh!

Xynoclafe commented Mar 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xynoclafe commented Mar 12, 2021

Uh oh!

Xynoclafe commented Mar 12, 2021

Uh oh!

Xynoclafe commented Mar 16, 2021

Uh oh!

wyunchi-ms commented Mar 17, 2021

Uh oh!

azure-pipelines bot commented Mar 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Xynoclafe commented Feb 24, 2021 •

edited

Loading